Skip to content
This repository has been archived by the owner on Oct 15, 2020. It is now read-only.

Update TTD for locations where AsyncHooks passes hidden state. #252

Closed
wants to merge 1 commit into from
Closed

Update TTD for locations where AsyncHooks passes hidden state. #252

wants to merge 1 commit into from

Conversation

mrkmarron
Copy link
Contributor

Affected core subsystem(s)

TTD, AsyncHooks

Description of change

AsyncHooks communicates between JS/Native using uncontrolled pointer writes. Add calls to notify TTD of these events.

@@ -1,4 +1,4 @@
// Copyright Joyent, Inc. and other Node contributors.
// Copyright Joyent, Inc. and other Node contributors.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is a BOM getting added or removed here?

@mrkmarron
Copy link
Contributor Author

Hi Kyle, I cleaned up the BOM (looking) issue and updated the PR.

#if ENABLE_TTD_NODE
if (s_doTTRecord || s_doTTReplay) {
unsigned int refcount = 0;
JsAddRef(*farray, &refcount);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is AsyncWrap a singleton? If not, is this a memory leak since I never see a JsRelease anywhere?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it is a singleton, nevermind.

Copy link
Contributor

@kfarnung kfarnung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants